Feat/group task orchestration#100
Open
ginccc wants to merge 12 commits into
Open
Conversation
- Fix TaskBoard flash on stream completion (show until API data loads) - Add ARIA attributes (role=progressbar, aria-valuenow, region labels) - Add section heading with ClipboardList icon to TaskBoard - Move pulse-border keyframe from inline style to index.css - Memoize API-loaded task board (MemoizedApiTaskBoard component) - Fix tasksCompleted inflation (exclude VERIFIED from completed set) - Fix SSE parser default event type (null instead of group_start) - Protect shared initialState Sets/Maps from accidental mutation - Add missing i18n keys (emptyState, unassigned) to all 11 locales - Use CSS custom properties for theme-aware animation colors
- Add 22 tests for TaskBoard component (empty state, columns, progress bar, priorities, verification, accessibility) - Add 6 tests for SSE hook task events (task_plan_created, task_verified, in-progress tracking, completion, reset, phase guard)
…trings - Fix fragile task-to-agent matching: prefer agentId over displayName with graceful fallback (use-group-discussion-stream.ts, groups.ts) - Add console.warn to all 9 silent catch blocks in SSE event handlers - Replace hardcoded strings with t() calls: LIVE indicator, HTML toggle, flow step labels, entry type labels, Group/Mod badges, lifecycle policies, task count interpolation (3 component files) - Add dotColor field to STYLE_THEME, removing brittle .replace() hack - Add 40+ new i18n keys across groups.* namespace (entryType, flow, lifecycle) to all 11 locale files
📝 WalkthroughWalkthroughAdds task-force discussion contracts, streaming state, task board rendering, configuration UI, localized labels, and test fixtures for the new task plan and verification flow. ChangesTask-force discussion flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds first-class support for the new TASK_FORCE group discussion style, including SSE-driven task orchestration state, a kanban-style Task Board in the transcript UI, and broader i18n coverage for group-related labels and entry types.
Changes:
- Introduces TASK_FORCE style across API types, UI theming, phase/entry mappings, and group state handling.
- Adds a Task Board component and wires it into the discussion transcript for both streaming and API-loaded conversations.
- Extends SSE stream state + tests to track task plan creation, execution progress, completion, and verification.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/api/groups.ts | Extends group API types (TASK_FORCE, new phases/entry types, task models) and SSE payload/event definitions. |
| src/hooks/use-group-discussion-stream.ts | Tracks task plan + verification + inferred execution progress during SSE streaming. |
| src/hooks/tests/use-group-discussion-stream.test.ts | Adds coverage for new SSE payload fields and task tracking behavior. |
| src/components/groups/task-board.tsx | New Task Board UI (columns, cards, progress bar) driven by SSE/API-derived task state. |
| src/components/groups/tests/task-board.test.tsx | Adds extensive component tests for Task Board rendering and status bucketing. |
| src/components/groups/discussion-transcript.tsx | Integrates Task Board into transcript (streaming + API-loaded) and updates style theming/flow i18n. |
| src/components/groups/agent-response-card.tsx | Adds TASK_FORCE entry styling/icons and translates entry-type labels. |
| src/components/groups/phase-header.tsx | Adds icons for PLAN/EXECUTE/VERIFY phases. |
| src/components/groups/group-config-panel.tsx | Displays pre-configured tasks + dynamic agent settings and adds i18n wrapping for some labels. |
| src/pages/group-wizard.tsx | Adds TASK_FORCE style color mapping in the wizard. |
| src/pages/group-detail.tsx | Adds UI color mapping for the new AWAITING_APPROVAL state. |
| src/test/mocks/handlers.ts | Updates MSW mock group conversation objects with newly required task/dynamic-agent fields. |
| src/components/groups/tests/discussion-transcript.test.tsx | Updates transcript test fixtures with newly required conversation/stream fields. |
| src/index.css | Adds keyframes for Task Board “active card” pulse border animation. |
| src/i18n/locales/en.json | Adds group/task-board translation keys (styles, phases, entry types, task board labels). |
| src/i18n/locales/de.json | Propagates new group/task-board translation keys. |
| src/i18n/locales/fr.json | Propagates new group/task-board translation keys. |
| src/i18n/locales/es.json | Propagates new group/task-board translation keys. |
| src/i18n/locales/ar.json | Propagates new group/task-board translation keys. |
| src/i18n/locales/zh.json | Propagates new group/task-board translation keys. |
| src/i18n/locales/th.json | Propagates new group/task-board translation keys. |
| src/i18n/locales/pt.json | Propagates new group/task-board translation keys. |
| src/i18n/locales/ko.json | Propagates new group/task-board translation keys. |
| src/i18n/locales/ja.json | Propagates new group/task-board translation keys. |
| src/i18n/locales/hi.json | Propagates new group/task-board translation keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…n dynamic agent limits - Pass done/total to taskBoard.progress t() call (fixes raw placeholder) - Pass done/total to aria-label on progress bar (accessibility fix) - Use task.subject+idx as stable React key instead of array index - Wrap dynamic agent max strings in t() with interpolation - Add dynamicMax/dynamicMaxPerTask i18n keys to all 11 locales
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (3)
src/lib/api/groups.ts (1)
481-482: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant yield condition.
Since the left operand
eventTypealready gates the branch,(eventData || eventType)is always truthy and can be dropped — this is equivalent toif (eventType). Simplifying matches the stated intent ("only yield events with an explicitevent:type").♻️ Suggested simplification
- // Only yield events with an explicit event: type (skip bare data-only chunks) - if (eventType && (eventData || eventType)) { + // Only yield events with an explicit event: type (skip bare data-only chunks) + if (eventType) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/api/groups.ts` around lines 481 - 482, The yield guard in groups parsing is redundant because `eventType` already decides the branch, so simplify the condition in the event-stream handling logic to check only for an explicit `event:` type. Update the `eventType` conditional in the relevant parser block so it matches the intent of only yielding typed events and removes the unnecessary `eventData || eventType` part.src/hooks/use-group-discussion-stream.ts (1)
327-341: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant identity mapping over
payload.tasks.The
maponly spreads each task and re-assignsassignedAgentIdto itself, producing an equivalent array. You can storepayload.tasksdirectly (the element shape already matchestaskPlan).♻️ Suggested simplification
- setState((s) => ({ - ...s, - taskPlan: payload.tasks.map((t) => ({ - ...t, - assignedAgentId: t.assignedAgentId, - })), - })); + setState((s) => ({ + ...s, + taskPlan: payload.tasks, + }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/use-group-discussion-stream.ts` around lines 327 - 341, The task_plan_created handler in useGroupDiscussionStream is doing a redundant identity map over payload.tasks by spreading each task and reassigning assignedAgentId to itself. Simplify the state update by storing payload.tasks directly in taskPlan, keeping the existing JSON.parse and console.warn error handling intact.src/components/groups/__tests__/task-board.test.tsx (1)
137-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
data-testidhooks over class-name / DOM-structure assertions.These assertions couple the tests to Tailwind class names and DOM shape:
querySelector(".animate-spin"),feedbackEl.className.toMatch(/emerald/)//destructive/, and.parentElementtraversal. A styling refactor (e.g. the gradient/class fix flagged intask-board.tsx) would break these tests without any behavior change. Consider exposing intent via stabledata-testids (e.g. atask-board-spinnerand atask-card-feedbackwith adata-passedattribute) and asserting on those instead.As per path instructions: "Assert on
data-testidattributes in unit tests instead of relying on rendered text or DOM structure".Also applies to: 181-184, 209-213
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/groups/__tests__/task-board.test.tsx` around lines 137 - 141, The task-board tests are asserting on Tailwind classes and DOM structure instead of stable test hooks, which makes them brittle. Update the affected checks in task-board.test.tsx to use explicit data-testid or other intent-revealing attributes exposed by the task board and task card components, and replace the .animate-spin, className regex, and parentElement-based assertions with direct assertions against those stable hooks. Use the existing task-board-progress test target plus new identifiers like task-board-spinner and task-card-feedback (or equivalent symbols added in the components) so the tests verify behavior without depending on styling or layout.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/groups/task-board.tsx`:
- Around line 238-241: The progress fill in the task board uses an outdated
Tailwind background utility, so update the gradient class on the inline progress
bar div in task-board.tsx from bg-gradient-to-r to bg-linear-to-r. Locate the
element by the width style using pct in the task board component and keep the
rest of the gradient and sizing classes unchanged.
In `@src/i18n/locales/ar.json`:
- Around line 1392-1469: The new task-force UI strings in ar.json are still
English, so the Arabic locale is incomplete. Replace the added
task-board/task-force values in the ar locale with proper Arabic translations,
using the same keys and structure already present in the locale object. Make
sure the keys from the new block, including task-state, lifecycle,
dynamic-agent, and taskBoard entries, are localized consistently with the rest
of the file.
In `@src/i18n/locales/de.json`:
- Around line 1394-1469: The German locale entries for the new task-force UI are
still using English text, so translate the newly added strings in the locale
object that contains state, flow, lifecycle, and taskBoard labels. Update the
matching keys in the German JSON to proper German values, using the existing
translation structure and symbols like entryType, flow, lifecycle, and taskBoard
as anchors. Also make sure these keys stay aligned with the base English locale
and are propagated consistently across the other locale files added for the same
feature set.
In `@src/i18n/locales/es.json`:
- Around line 1393-1469: The new task-force and task-board strings in the
Spanish locale are still English, so update the affected keys in the locale
object (including entryType, flow, lifecycle, taskBoard, and related labels) to
proper Spanish translations instead of copying the source text. Use the existing
i18n key names in es.json as the reference point, and make sure any newly added
keys are kept consistent with en.json and the other locale files so the UI does
not render mixed-language copy.
In `@src/i18n/locales/fr.json`:
- Around line 1393-1453: The new task-force localization entries are still using
English placeholders in the French locale, so update the added groups.* and
taskBoard.* keys in fr.json to proper French translations. Mirror the same key
set added in en.json across the locale object where these strings live, using
the existing translation patterns in related sections like flow and lifecycle as
a guide.
In `@src/i18n/locales/hi.json`:
- Around line 1392-1453: The Hindi locale block still contains English
placeholders for the new task-force flow, so translate the newly added keys to
Hindi in hi.json and keep the wording consistent with the existing locale style.
Update the full set of symbols in this section, including maxRounds,
styleTaskForce, phasePlan/phaseExecute/phaseVerify, dynamicAgents and related
dynamic* labels, entryType, flow, lifecycle, and the dynamicMax strings, so the
locale matches the new keys added in en.json.
In `@src/i18n/locales/ja.json`:
- Around line 1392-1453: The Japanese locale update is incomplete: new
flow-related keys added in en.json are still using English placeholders in the
ja.json translation block. Update the corresponding entries in the ja.json
locale object for the task-force labels, phases, flow names, and task-board copy
so they are properly localized and match the existing key structure used by the
locale files.
In `@src/i18n/locales/ko.json`:
- Around line 1392-1453: The new Korean locale entries are still left in
English, so update the newly added translation keys for the task-force UI to
match the rest of the locale file. Propagate the `groups` and `taskBoard`
strings from `en.json` into the Korean locale and translate them consistently
with the existing `entryType`, `flow`, and `lifecycle` sections so `ko.json`
stays aligned with the other locale files.
In `@src/i18n/locales/pt.json`:
- Around line 1392-1453: The Portuguese locale bundle is still using English
strings for the new task-force UI, so mirror the new keys from the en.json
update into the pt.json translation object. Update the affected task-force
labels and board/status strings in the locale structure that contains maxRounds,
preConfiguredTasks, entryType, flow, lifecycle, and the dynamicMax fields,
keeping the same key names and nesting.
In `@src/i18n/locales/th.json`:
- Around line 1392-1453: The new task-board and flow keys in th.json are still
using English placeholders, so replace them with real Thai translations for the
same locale keys added in en.json. Update the matching entries under entryType,
flow, lifecycle, and the related task-board strings (such as maxRounds,
dynamicAgents, preConfiguredTasksCount, dynamicMax, and dynamicMaxPerTask) so
Thai users see localized copy. Keep the key structure and names unchanged to
match the other locale files and the source strings.
In `@src/i18n/locales/zh.json`:
- Around line 1392-1469: The new task-force/task-board localization strings in
zh.json are still in English, so update those keys with proper Chinese
translations and keep the key set aligned with the rest of the locales. Use the
task-board/configuration entries in the same JSON block (for example maxRounds,
styleTaskForce, phaseExecute, taskBoard.title, taskBoard.emptyTitle) as the
reference set, and ensure any new i18n keys were first added to en.json before
mirroring them across all locale files.
---
Nitpick comments:
In `@src/components/groups/__tests__/task-board.test.tsx`:
- Around line 137-141: The task-board tests are asserting on Tailwind classes
and DOM structure instead of stable test hooks, which makes them brittle. Update
the affected checks in task-board.test.tsx to use explicit data-testid or other
intent-revealing attributes exposed by the task board and task card components,
and replace the .animate-spin, className regex, and parentElement-based
assertions with direct assertions against those stable hooks. Use the existing
task-board-progress test target plus new identifiers like task-board-spinner and
task-card-feedback (or equivalent symbols added in the components) so the tests
verify behavior without depending on styling or layout.
In `@src/hooks/use-group-discussion-stream.ts`:
- Around line 327-341: The task_plan_created handler in useGroupDiscussionStream
is doing a redundant identity map over payload.tasks by spreading each task and
reassigning assignedAgentId to itself. Simplify the state update by storing
payload.tasks directly in taskPlan, keeping the existing JSON.parse and
console.warn error handling intact.
In `@src/lib/api/groups.ts`:
- Around line 481-482: The yield guard in groups parsing is redundant because
`eventType` already decides the branch, so simplify the condition in the
event-stream handling logic to check only for an explicit `event:` type. Update
the `eventType` conditional in the relevant parser block so it matches the
intent of only yielding typed events and removes the unnecessary `eventData ||
eventType` part.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8069c759-baae-4498-bbaa-a1f31a36b931
📒 Files selected for processing (25)
src/components/groups/__tests__/discussion-transcript.test.tsxsrc/components/groups/__tests__/task-board.test.tsxsrc/components/groups/agent-response-card.tsxsrc/components/groups/discussion-transcript.tsxsrc/components/groups/group-config-panel.tsxsrc/components/groups/phase-header.tsxsrc/components/groups/task-board.tsxsrc/hooks/__tests__/use-group-discussion-stream.test.tssrc/hooks/use-group-discussion-stream.tssrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/index.csssrc/lib/api/groups.tssrc/pages/group-detail.tsxsrc/pages/group-wizard.tsxsrc/test/mocks/handlers.ts
…ogic, translate all locales - Replace bg-gradient-to-r with bg-linear-to-r (Tailwind v4 syntax) - Simplify redundant yield condition in SSE parser (groups.ts) - Remove identity mapping in task_plan_created handler - Translate all groups.* and taskBoard.* keys for 10 locales: de, fr, es, pt, ar, zh, ja, ko, th, hi
… label fix - Normalise CRLF to LF in SSE parser so \r\n\r\n boundaries are detected - Support both groupConversationId and conversationId in group_start payload for backwards compatibility with older backends - Fix DELPHI flow label: 'Anonymous' → 'Anonymous Sharing' to match the groups.flow.AnonymousSharing i18n key
- Groups overview card: show member preview chips with Bot/Group icons - Groups overview table: replace ID/Version columns with Style/Members - Table rows: fully clickable with keyboard accessibility (Enter/Space) - Conversation sidebar: new entry appears immediately on stream start (invalidate on IN_PROGRESS, not just COMPLETED) - Conversation sidebar: show LIVE badge with pulse indicator on streaming - State labels: human-friendly translated text instead of raw enums (e.g. 'In Progress' not 'IN_PROGRESS') - Accessibility: aria-label, aria-live, aria-current, role=list/listitem, tabIndex, keyboard navigation, aria-hidden on icons - i18n: add styleColumn, membersColumn, memberOverflow, defaultLabel, liveDiscussion, state.* keys to all 11 locales - Tests: 79 new/updated tests across 3 files (group-card, groups, group-detail)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/pages/__tests__/groups.test.tsx (1)
131-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSwitch these table-view assertions to stable testids.
The new coverage is still tied to localized text and DOM shape (
getByText("Style"), role/class checks, direct querying). That makes the tests fragile as copy and presentation evolve. Please preferdata-testid-based assertions for the new headers and row metadata. As per coding guidelines,Assert on \data-testid` attributes in unit tests instead of relying on rendered text or DOM structure`.Also applies to: 465-623
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/__tests__/groups.test.tsx` around lines 131 - 137, The new group table assertions in groups.test.tsx are still relying on rendered text and DOM structure, which makes them brittle. Update the coverage around the table headers and row metadata to use stable data-testid-based queries/assertions instead of getByText and role/class-based checks. Keep the existing test intent, but locate the affected cases in the new table-view coverage and switch them to the relevant testid hooks so they remain stable as copy and markup change.Source: Coding guidelines
src/pages/__tests__/group-detail.test.tsx (1)
207-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid asserting localized copy and Tailwind classes for conversation state.
These checks are now coupled to English labels (
"Completed","In Progress","Failed") and presentation classes liketext-destructive/rounded-full. Please assert through the existing state testids instead, and expose a stable data attribute if you need to verify the mapped state value. As per coding guidelines,Assert on \data-testid` attributes in unit tests instead of relying on rendered text or DOM structure`.Also applies to: 308-381
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/__tests__/group-detail.test.tsx` at line 207, The conversation state tests are asserting localized labels and Tailwind styling, which makes them brittle and tied to presentation. Update the group detail tests to use the existing state testids instead of matching text like “Completed”, “In Progress”, or “Failed”, and avoid checking classes such as text-destructive or rounded-full. If the mapped state value still needs verification, add a stable data attribute on the relevant state-rendering element and assert against that in the affected tests in group-detail.test.tsx.Source: Coding guidelines
src/components/groups/__tests__/group-card.test.tsx (1)
137-259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer testid-based assertions for these new cases.
These additions are mostly pinned to translated copy and raw DOM structure (
getByText,querySelector("svg"), link traversal), which will churn on locale or markup-only changes. Please anchor them to explicitdata-testidhooks instead. As per coding guidelines,Assert on \data-testid` attributes in unit tests instead of relying on rendered text or DOM structure`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/groups/__tests__/group-card.test.tsx` around lines 137 - 259, The new GroupCard tests are too coupled to translated text and raw DOM structure, so switch them to explicit data-testid-based assertions. Update the cases in GroupCard’s test suite to target existing or added testid hooks for style badges, member chips, overflow text, the group name link, and relative time instead of getByText, closest("a"), or querySelector("svg"). Keep the assertions anchored to stable symbols like GroupCard, renderWithProviders, and the member/list elements via data-testid.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/groups.tsx`:
- Around line 197-208: The row-level navigation in the groups table is still
handling Enter/Space from nested controls, so keyboard interaction on the inner
Link and duplicate/delete buttons can trigger row navigation. Update the row
onKeyDown handler and the nested control handlers in the groups row markup to
ignore events originating from interactive descendants, matching the existing
stopPropagation behavior used by onClick. Use the row wrapper and the nested
Link/button elements in the group row render paths to keep keyboard activation
scoped to the intended control.
---
Nitpick comments:
In `@src/components/groups/__tests__/group-card.test.tsx`:
- Around line 137-259: The new GroupCard tests are too coupled to translated
text and raw DOM structure, so switch them to explicit data-testid-based
assertions. Update the cases in GroupCard’s test suite to target existing or
added testid hooks for style badges, member chips, overflow text, the group name
link, and relative time instead of getByText, closest("a"), or
querySelector("svg"). Keep the assertions anchored to stable symbols like
GroupCard, renderWithProviders, and the member/list elements via data-testid.
In `@src/pages/__tests__/group-detail.test.tsx`:
- Line 207: The conversation state tests are asserting localized labels and
Tailwind styling, which makes them brittle and tied to presentation. Update the
group detail tests to use the existing state testids instead of matching text
like “Completed”, “In Progress”, or “Failed”, and avoid checking classes such as
text-destructive or rounded-full. If the mapped state value still needs
verification, add a stable data attribute on the relevant state-rendering
element and assert against that in the affected tests in group-detail.test.tsx.
In `@src/pages/__tests__/groups.test.tsx`:
- Around line 131-137: The new group table assertions in groups.test.tsx are
still relying on rendered text and DOM structure, which makes them brittle.
Update the coverage around the table headers and row metadata to use stable
data-testid-based queries/assertions instead of getByText and role/class-based
checks. Keep the existing test intent, but locate the affected cases in the new
table-view coverage and switch them to the relevant testid hooks so they remain
stable as copy and markup change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 285377f9-1a89-4dc7-a744-b81be79d3a41
📒 Files selected for processing (20)
src/components/groups/__tests__/group-card.test.tsxsrc/components/groups/discussion-transcript.tsxsrc/components/groups/group-card.tsxsrc/hooks/use-group-discussion-stream.tssrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/lib/api/groups.tssrc/pages/__tests__/group-detail.test.tsxsrc/pages/__tests__/groups.test.tsxsrc/pages/group-detail.tsxsrc/pages/groups.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- src/i18n/locales/ja.json
- src/i18n/locales/en.json
- src/i18n/locales/ar.json
- src/i18n/locales/de.json
- src/i18n/locales/th.json
- src/i18n/locales/ko.json
- src/i18n/locales/es.json
- src/i18n/locales/hi.json
- src/i18n/locales/fr.json
- src/i18n/locales/pt.json
- src/i18n/locales/zh.json
- src/hooks/use-group-discussion-stream.ts
- src/components/groups/discussion-transcript.tsx
- src/lib/api/groups.ts
- Add Task Force preset to group-templates.ts (🎯 icon, TASK_FORCE style) - Roles: Project Lead, Researcher, Implementer, Quality Assurance - Add i18n keys (taskForce, taskForceDesc, 4 role names) to all 11 locales - Wizard now shows 6 templates instead of 5
- Add aria-label to card context menu button - Add aria-label to table duplicate/delete buttons - Add aria-label to conversation sidebar delete button - Use members.length for table count (consistent with card view)
- Expand from 3 to 8 mock groups covering all 6 discussion styles - Each group has unique members, roles, and realistic descriptions - Groups grp7 and grp8 include GROUP-type members (nested groups) - Per-group conversation history (1-3 conversations each, varied states) - Add TASK_FORCE to styles mock endpoint - GET handler now returns per-ID config instead of single hardcoded response - Fix empty-state tests with server.use override
- Extract DEFAULT_STATE constant so fallback is a definite type - Removes undefined possibility from Record<string, T> indexed access - Remove debug console.log from MSW handlers
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/group-templates.ts`:
- Around line 107-116: The task-force template strings in groupTemplates are
using bare t(...) lookups, so missing locale entries will show raw keys in the
UI. Update the task-force block in the group template definition to use inline
fallback values for the name, description, and each role label, keeping the
existing translation keys but providing readable defaults when translations are
absent.
In `@src/test/mocks/handlers.ts`:
- Around line 2081-2083: The mock handlers are falling back to default data for
unknown group IDs, which hides invalid route params in tests. Update the
relevant handler logic in handlers.ts around the group config lookup and the
group list lookup so they validate the requested id against the available mock
keys and return an explicit 404 response when the id is not found, instead of
defaulting to grp1 or an empty array.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 76f124cd-6f9d-4084-97fc-489e5021bbac
📒 Files selected for processing (17)
src/components/groups/group-card.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/lib/group-templates.tssrc/pages/__tests__/group-detail.test.tsxsrc/pages/group-detail.tsxsrc/pages/groups.tsxsrc/test/mocks/handlers.ts
✅ Files skipped from review due to trivial changes (2)
- src/i18n/locales/hi.json
- src/i18n/locales/es.json
🚧 Files skipped from review as they are similar to previous changes (7)
- src/pages/tests/group-detail.test.tsx
- src/components/groups/group-card.tsx
- src/i18n/locales/de.json
- src/pages/group-detail.tsx
- src/i18n/locales/th.json
- src/i18n/locales/pt.json
- src/pages/groups.tsx
Comment on lines
+107
to
+116
| name: t("groupTemplates.taskForce"), | ||
| description: t("groupTemplates.taskForceDesc"), | ||
| icon: "🎯", | ||
| style: "TASK_FORCE", | ||
| maxRounds: 1, | ||
| roles: [ | ||
| { displayName: t("groupTemplates.roles.projectLead"), role: "Lead" }, | ||
| { displayName: t("groupTemplates.roles.researcher"), role: "Research" }, | ||
| { displayName: t("groupTemplates.roles.implementer"), role: "Implementation" }, | ||
| { displayName: t("groupTemplates.roles.qualityAssurance"), role: "QA" }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the changed block
FILE="src/lib/group-templates.ts"
nl -ba "$FILE" | sed -n '90,130p'
# Look for i18n usage patterns with fallbacks in the repo
rg -n 't\("([^"]+)",\s*"[^"]+"\)' src --glob '*.{ts,tsx}' | sed -n '1,120p'
# Look for bare t(...) calls in similar files for context
rg -n 't\("([^"]+)"\)' src/lib --glob '*.{ts,tsx}' | sed -n '1,160p'Repository: labsai/EDDI-Manager
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/lib/group-templates.ts"
# Show the relevant section with portable line numbering
sed -n '90,130p' "$FILE" | cat -n
# Show all i18n calls in this file
rg -n 't\("' "$FILE"
# Show nearby uses of t(...) in related code for fallback style
rg -n 't\(".*?",\s*".*?"\)' src --glob '*.{ts,tsx}' | sed -n '1,120p'Repository: labsai/EDDI-Manager
Length of output: 17180
Add inline fallbacks to the new group-template translations. The task-force block adds bare t(...) calls for the template name, description, and role labels; missing locale entries will surface raw keys in the UI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/group-templates.ts` around lines 107 - 116, The task-force template
strings in groupTemplates are using bare t(...) lookups, so missing locale
entries will show raw keys in the UI. Update the task-force block in the group
template definition to use inline fallback values for the name, description, and
each role label, keeping the existing translation keys but providing readable
defaults when translations are absent.
Source: Coding guidelines
Comment on lines
+2081
to
+2083
| const id = params.id as string; | ||
| const config = groupConfigs[id] ?? groupConfigs.grp1; | ||
| return HttpResponse.json(config); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return 404s for unknown mock IDs.
Line 2082 falls back to grp1, and Line 2151 falls back to []. That makes bad route params look valid in tests, so wiring bugs can slip through unnoticed. Prefer an explicit 404 for unknown group ids.
Suggested fix
http.get("*/groupstore/groups/:id", ({ params }) => {
const groupConfigs: Record<string, object> = {
// ...
};
const id = params.id as string;
- const config = groupConfigs[id] ?? groupConfigs.grp1;
- return HttpResponse.json(config);
+ const config = groupConfigs[id];
+ if (!config) {
+ return HttpResponse.json({ error: `Unknown group id: ${id}` }, { status: 404 });
+ }
+ return HttpResponse.json(config);
});
http.get("*/groups/:groupId/conversations", ({ params }) => {
const now = Date.now();
const groupConversations: Record<string, object[]> = {
// ...
};
const id = params.groupId as string;
- return HttpResponse.json(groupConversations[id] ?? []);
+ const conversations = groupConversations[id];
+ if (!conversations) {
+ return HttpResponse.json({ error: `Unknown group id: ${id}` }, { status: 404 });
+ }
+ return HttpResponse.json(conversations);
}),Also applies to: 2150-2151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/mocks/handlers.ts` around lines 2081 - 2083, The mock handlers are
falling back to default data for unknown group IDs, which hides invalid route
params in tests. Update the relevant handler logic in handlers.ts around the
group config lookup and the group list lookup so they validate the requested id
against the available mock keys and return an explicit 404 response when the id
is not found, instead of defaulting to grp1 or an empty array.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces support for the new "TASK_FORCE" discussion style, including UI theming, a dynamic task board, and improved handling of task-related transcript entries. It also enhances internationalization by wrapping various labels and UI strings with translation functions, and improves the visual cues for discussion progress and agent responses.
TASK_FORCE Style and Task Board Integration
STYLE_THEME), flow steps, and phase mapping. [1] [2] [3] [4] [5] [6] [7] [8] [9]TaskBoardcomponent into theDiscussionTranscriptfor "TASK_FORCE" discussions, displaying during streaming and when loaded from the API. Includes a memoized wrapper for API-loaded data. [1] [2]taskPlan,taskVerifications,tasksInProgress,tasksCompleted,taskList). [1] [2] [3]Transcript Entry and Agent Response Enhancements
AgentResponseCardto show custom icons for plan and verification entries, and to support translation of entry type labels. [1] [2]Internationalization Improvements
t()), including state labels, live indicator, HTML toggle, and flow steps. [1] [2] [3] [4] [5]Visual and Theming Updates
dotColorproperty toSTYLE_THEMEfor consistent progress indicator coloring across discussion styles. Updated progress indicators to use this property. [1] [2] [3] [4] [5] [6] [7] [8]These changes collectively enable a richer and more flexible group discussion experience, especially for collaborative, task-oriented workflows.
Summary by CodeRabbit